Skip to content

e2e tests: use ginkgo.DeferCleanup for consistent resource cleanup#1705

Open
ErikJiang wants to merge 1 commit into
llm-d:mainfrom
ErikJiang:use_ginkgo_defercleanup
Open

e2e tests: use ginkgo.DeferCleanup for consistent resource cleanup#1705
ErikJiang wants to merge 1 commit into
llm-d:mainfrom
ErikJiang:use_ginkgo_defercleanup

Conversation

@ErikJiang

Copy link
Copy Markdown
Contributor

What type of PR is this?

/kind cleanup

What this PR does / why we need it:

Use ginkgo.DeferCleanup consistently across all e2e tests instead of manually calling testutils.DeleteObjects at the end of each test.

This ensures resources are cleaned up even when a test fails before reaching the cleanup code, preventing resource leaks and test pollution.

Which issue(s) this PR fixes:

Fixes #

Release note (write NONE if no user-facing change):

NONE

@ErikJiang
ErikJiang requested a review from a team as a code owner June 22, 2026 10:30
@ErikJiang
ErikJiang requested review from elevran and liu-cong June 22, 2026 10:30
@github-actions github-actions Bot added kind/bug Categorizes issue or PR as related to a bug. size/L Denotes a PR that changes 100-499 lines, ignoring generated files. kind/cleanup kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. labels Jun 22, 2026
@ahg-g

ahg-g commented Jun 27, 2026

Copy link
Copy Markdown
Collaborator

sorry, can you pls resolve the conflicts

Signed-off-by: bo.jiang <bo.jiang@daocloud.io>
@ErikJiang
ErikJiang force-pushed the use_ginkgo_defercleanup branch from 895c168 to 309cdd9 Compare June 29, 2026 02:20
@ErikJiang

Copy link
Copy Markdown
Contributor Author

Done. Conflicts resolved. PTAL. @ahg-g

@shmuelk

shmuelk commented Jul 8, 2026

Copy link
Copy Markdown
Contributor

@ErikJiang I am working on making the E2E tests run in parallel. To simplify the code, the tests are wrapped with a wrapper that does per group of tests (basically under a ginkgo.When block). The wrapper uses ginkgo.BeforeAll and ginkgo.AfterAll calls to perform standard per test group (i.e. namespace) setup and teardown. This doesn't work well with using ginkgo.DeferedCleanup.

@elevran

elevran commented Jul 14, 2026

Copy link
Copy Markdown
Collaborator

@shmuelk - is this PR (or some other form of it) needed after the move to parallel e2e?
How would it change to support cleanup on early test exit when multiple tests run in parallel?

@shmuelk

shmuelk commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

As I commented above the use of DeferredCleanup has to be used very carefully with the new parallel tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

kind/bug Categorizes issue or PR as related to a bug. kind/cleanup kind/documentation Categorizes issue or PR as related to documentation. kind/feature Categorizes issue or PR as related to a new feature. size/L Denotes a PR that changes 100-499 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants